Redesign Portfolio tab v2 — remove all toggles#719
Conversation
- Remove all <details>/collapse/toggle buttons from Portfolio tab - Donations given: always visible with full per-story breakdown - Trading history: always visible, no collapsible wrapper - All info displayed directly: portfolio summary, holdings, donations, trades - Consistent card styling throughout Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE
Minimal, surgical change — simply swaps <details>/<summary> to <div> in two places. All #710 criteria met:
- ✅ Donations given:
<details>→<div>, toggle arrows removed — always visible - ✅ Trading history:
<details>→<div>, toggle arrows removed — always visible - ✅ Zero toggle/collapse elements remaining
- ✅ All info directly visible: portfolio, holdings, donations, trades
- ✅ Layout unchanged otherwise — mobile styling preserved
- ✅ Build passes
+8/−12, single file. No concerns.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The toggles are gone, but the code still does not satisfy issue #710's mobile-first redesign requirement because the densest rows remain locked to single-line desktop-style layouts.
Findings
- [medium] The donations-given rows and trading-history rows are still rigid
justify-betweensingle-line flex layouts with non-wrapping right-hand value clusters. On a 375px viewport, the amount + USD + tx-link segment cannot wrap and the row containers themselves do not wrap, so the redesign still risks the same cramped/overflow behavior the issue calls out. This PR removes the toggles, but it does not actually redesign the mobile row structure.- File:
src/app/profile/[address]/page.tsx:1522 - Suggestion: Stack or wrap these rows on mobile, or move secondary metadata under the primary line so donations/trades can breathe at narrow widths.
- File:
- [medium] The same mobile constraint problem exists in trading history: the outer row stays
flex items-center justify-between, the right cost cluster isshrink-0, and token/date metadata also stays inline. That means long titles already truncate and the remaining inline metadata still competes for width on small screens instead of adapting to a mobile-first layout.- File:
src/app/profile/[address]/page.tsx:1651 - Suggestion: Use a two-line or stacked mobile layout for each trade row, then enhance to the denser inline arrangement at larger breakpoints.
- File:
Decision
Requesting changes because the current patch removes the accordion controls but does not complete the mobile-friendly portfolio redesign required by issue #710.
Both donation-given rows and trade-history rows now use flex-wrap with gap-based spacing instead of rigid justify-between, preventing overflow at 375px. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up revision completes the mobile-first cleanup by removing the remaining rigid row structures after the toggle removal.
Findings
- None.
Decision
Donations-given rows and trading-history rows now use wrapping, gap-based layouts instead of fixed justify-between splits, so the dense portfolio metadata can adapt cleanly on narrow screens while keeping all information visible. I am approving.
Summary
<details>)<div>instead of<details>Fixes #710
Self-Verification
npm run buildpasses🤖 Generated with Claude Code